fix(mcp): preserve provider policy receipts - #10573
Conversation
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueComment |
|
PR Review Advisor finished for commit |
<!-- markdownlint-disable MD041 --> ## Outcome Maintainers can inspect a pull request's complete retained lifetime in Perfetto from the standard value-stream analysis command. The command now writes a validated trace, matching summary, and manifest instead of limiting interactive evidence to a static latest-revision waterfall. ## Reason The existing bounded JSON summarized bottlenecks but could not expose concurrent workflows, runner queues, individual steps, lifecycle events, or contributor feedback loops interactively. A single canonical lifetime trace makes those relationships explorable without adding user-selected analysis modes. ## Changes - Collect bounded lifecycle events, comments, inline feedback, every retained PR revision, matching workflow jobs and steps, and external checks during the standard analyzer invocation. - Render Chrome Trace Event JSON with independent Perfetto tracks for workflows, jobs, steps, author publication, feedback waiting, and response spans; validate phase, timestamp, metadata, and slice-nesting invariants before publishing the manifest. - Write summary.json, trace.json, and manifest.json atomically under the ignored maintainer artifact directory, failing closed on incomplete required history or a changed PR head. - Update the maintainer skill contract and focused process-boundary tests for the one complete lifetime workflow. ## Verification - Contributor validation: Signed commit passed pre-commit, commit-msg, and pre-push hooks, including repository checks, Oxfmt, Oxlint, markdownlint, gitleaks, source-shape budget, codebase growth guardrails, and CLI typecheck. - Tests: `npx vitest run --project integration test/automation/pull-requests/analyze-pr-value-stream.test.ts`: 19 tests passed. Real Perfetto-compatible traces validated for open draft PR #10573, open PR #10574, and merged PRs #10566 and #10575. - Broad gate: `npm run checks:repository` passed: 1,849 files, 5,878 edges, 0 cycles, and exact membership for 2,634 test candidates. - Secrets review: The diff contains no secrets, API keys, or credentials <!-- nemoclaw-docs-review:start --> - Documentation review: `docs-updated` - Documentation evidence: Maintainer skill documents portable stale-lock recovery behavior. - Documentation agent: openai/openai/gpt-5.6-sol <!-- docs-review-head-sha: e53b069 --> <!-- docs-review-agents-blob-sha: dd3528f --> <!-- nemoclaw-docs-review:end --> <!-- nemoclaw-targeted-validation:start --> - Targeted validation: Value-stream and growth integration: 66 passed; focused TypeScript and Oxlint passed. <!-- nemoclaw-targeted-validation:end --> <!-- nemoclaw-broad-gate:start --> - Broad gate: passed — Repository checks and signed commit hooks passed; all twenty-two GitHub commits are verified. <!-- nemoclaw-broad-gate:end --> --- Signed-off-by: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added complete pull request lifetime analysis covering commits, checks, reviews, comments, and lifecycle events. * Generated summary, trace, and manifest artifacts with event counts, timestamps, completeness details, and caveats. * Added Chrome Trace output for visualizing pull request activity over time. * Added safeguards for bounded analysis and consistent artifact publication. * **Bug Fixes** * Improved timestamp validation and artifact integrity checks. * Prevented incomplete or inconsistent artifacts from being retained after failures. * Added stale-lock recovery and protection against publishing results for outdated revisions. * **Tests** * Expanded coverage for trace validity, completeness, concurrent publishing, and lifecycle changes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Carlos Villela <cvillela@nvidia.com>
|
Closing this PR because the accepted policy decision in #10514 supersedes #9833. This PR preserves provider policy receipts, but #10514 requires OpenShell to be the sole durable policy authority and prohibits durable policy receipts and authority records. Replacement work is in #10515, which delivers the accepted cutover and states that no intermediate authority-stack PR is intended to merge. This closure records supersession. It does not assess the contributor or the implementation quality. |
Outcome
BLOCKED DRAFT — independent review has not approved this commit. Do not mark this PR ready for review or merge.
This draft preserves policy creation receipts across managed MCP provider attachment changes. It also keeps an incomplete add retryable when final policy-authority verification refuses the operation.
Reason
Concurrent MCP adds can rotate the live sandbox policy identity after NemoClaw records its policy creation receipt. The current failure can report two receipt-mismatch errors instead of one durable success and one refusal, and a failed add can leave state that makes the next attempt report
already exists.Related issues
Refs #9833
Changes
preflightedadd by compensating its recorded provider attachment before an ordinary retry.Verification
86c57c4ec0efe6adf0cf2d8463e9007e06b3e44e10e79fa88b865ba62b3229c7.git diff --check— passed.npm run test:changed— 6,167 passed and 2 skipped. Seven environment failures reproduced on the unmodified base: five uninstall cases stopped at the host OpenShell Homebrew pinned-trust check; one Hermes gateway drift case used system Python 3.9.6, which does not supportzip(strict=True); one Hermes adapter case used a system Python environment without PyYAML.dbd4e085678ed97562babcde24a45c8ed9e292a2— GitHub Verified, reasonvalid, with DCO sign-off.Review notes
This draft remains blocked on these independent-review findings:
This PR does not claim security approval or merge readiness. The commit is based on
fa6b2c89eb01e2b27648cfea61208a9897d5fcb4; refreshedorigin/mainis verified commit29e79e7163ea3837f90759c3ed93d54422eb6341, three commits ahead. Publication does not rebase or modify the frozen stacked PRs or worktrees.Signed-off-by: Apurv Kumaria akumaria@nvidia.com